Skip to content

boot-qemu.py: Check gdb_bin earlier#85

Merged
nathanchance merged 1 commit into
ClangBuiltLinux:mainfrom
nathanchance:fix-gdb-check
Dec 14, 2022
Merged

boot-qemu.py: Check gdb_bin earlier#85
nathanchance merged 1 commit into
ClangBuiltLinux:mainfrom
nathanchance:fix-gdb-check

Conversation

@nathanchance

@nathanchance nathanchance commented Dec 14, 2022

Copy link
Copy Markdown
Member

Otherwise, the call to sys.exit() in die() does not work and we end up
hanging.

Additionally, move it out of the while loop, as it does not make send to
check for gdb_bin every single time we want to invoke it.

@nathanchance
boot-qemu.py: Check gdb_bin earlier
772eb24 
Otherwise, the call to sys.exit() in die() does not work and we end up
hanging.

Additionally, move it out of the while loop, as it does not make send to
check for gdb_bin every single time we want to invoke it.

Reported-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
nickdesaulniers
nickdesaulniers approved these changes Dec 14, 2022
View reviewed changes

@nickdesaulniers nickdesaulniers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Though I'm still perplexed; why does sys.exit(1) not end the process? That still seems like a bug to me.

@nathanchance

nathanchance commented Dec 14, 2022

Copy link
Copy Markdown
Member Author

Though I'm still perplexed; why does sys.exit(1) not end the process? That still seems like a bug to me.

According to Python's documentation around sys.exit() and subprocess.Popen():

A call to sys.exit() is translated into an exception so that clean-up handlers (finally clauses of try statements) can be executed, and so that a debugger can execute a script without running the risk of losing control.

Popen objects are supported as context managers via the with statement: on exit, standard file descriptors are closed, and the process is waited for.

So what I suspect is happening here is that sys.exit() is called within the with statement, raising a SystemExit exception, and the interpreter waits for QEMU to exit before the script is allowed to exit. However, we spawned QEMU with -S, which freezes the CPU until it is started with GDB, so the interpreter hangs. I do not know if that is really a bug. We could potentially solve it by wrapping the whole with statement in a try statement. But this seems like a better fix in my opinion.

@nathanchance nathanchance merged commit 64b7d42 into ClangBuiltLinux:main Dec 14, 2022
@nathanchance nathanchance deleted the fix-gdb-check branch December 14, 2022 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickdesaulniers nickdesaulniers nickdesaulniers approved these changes

@msfjarvis msfjarvis msfjarvis approved these changes

@tpimh tpimh Awaiting requested review from tpimh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nathanchance @nickdesaulniers @msfjarvis